Skip to content

Require IDs to be unique (even with drafts) #3185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ddbeck
Copy link
Collaborator

@ddbeck ddbeck commented Jul 28, 2025

This is table setting for #2666 and #2339 and #2341.

Before this, it was possible to have a draft feature shadow the ID of a published feature. This change improves the safety of moving features from draft to published (and vice versa).

While we don't have draft groups or snapshots, I checked these too since it seemed weird and incomplete not too.

This is table setting for
web-platform-dx#2666 and
web-platform-dx#2339 and
web-platform-dx#2341.

Before this, it was possible to have a draft feature shadow the ID of a
published feature. This change improves the safety of moving features
from draft to published (and vice versa).

While we don't have draft groups or snapshots, I checked these too since
it seemed weird and incomplete not too.
@ddbeck ddbeck requested a review from foolip July 28, 2025 12:33
Copy link
Collaborator

@foolip foolip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this can be simplified by using data[draft], but if not it's good as is!


// Assert ID uniqueness
for (const [pool, map] of Object.entries(uniqueIdMaps)) {
if (!pathParts.includes("spec") && pathParts.includes(pool)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the !pathParts.includes("spec") condition about the features/draft/spec/ directory? If so, should we instead give all drafts the same treatment? We set data[draft] in this loop so we might be able to use that in the condition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants